src/main/webapp/i18n/en/buildAgents.json (2)

26-27: Consider rephrasing for better clarity.

The messages are informative but could be slightly improved for readability.

-                "buildAgentsPaused": "Pause request sent to all build agents. The agents will not accept new build jobs and will gracefully finish the current build jobs or will cancel them after a configurable seconds.",
+                "buildAgentsPaused": "Pause request sent to all build agents. Agents will not accept new build jobs and will gracefully finish current jobs or cancel them after the configured timeout.",
-                "buildAgentsResumed": "Resume request sent to all build agents. The agents will start accepting new build jobs."
+                "buildAgentsResumed": "Resume request sent to all build agents. Agents will now accept new build jobs."

---

31-33: Improve warning message formatting.

The warning message uses a raw newline character (\n). Consider using proper JSON string formatting for better maintainability.

-            "pauseAllWarning": "You are about to pause all build agents. This will prevent them from processing any new build jobs.\nAre you sure you want to proceed?"
+            "pauseAllWarning": "You are about to pause all build agents. This will prevent them from processing any new build jobs. Are you sure you want to proceed?"

src/main/webapp/i18n/de/buildAgents.json (2)

31-32: Fix capitalization inconsistency in button labels.

The capitalization is inconsistent between "Alle Agenten Anhalten" and "Alle Agenten fortsetzen". In German, both verbs should be capitalized when used as button labels.

  "pauseAll": "Alle Agenten Anhalten",
- "resumeAll": "Alle Agenten fortsetzen",
+ "resumeAll": "Alle Agenten Fortsetzen",

---

33-33: Consider using HTML line break for better rendering control.

While "\n" works for line breaks, using HTML <br> tags is more common in UI messages and provides better control over rendering across different platforms.

- "pauseAllWarning": "Du bist dabei, alle Build-Agenten anzuhalten. Dies wird verhindern, dass sie neue Build-Jobs verarbeiten.\nBist du sicher, dass du fortfahren möchtest?"
+ "pauseAllWarning": "Du bist dabei, alle Build-Agenten anzuhalten. Dies wird verhindern, dass sie neue Build-Jobs verarbeiten.<br>Bist du sicher, dass du fortfahren möchtest?"

src/main/webapp/app/localci/build-agents/build-agents.service.ts (2)

43-52: Add JSDoc return type documentation.

While the implementation is correct, consider enhancing the documentation with return type information.

    /**
     * Pause All Build Agents
+    * @returns Observable<void> Completes when all agents are paused
     */
    pauseAllBuildAgents(): Observable<void> {

    /**
     * Resume all Build Agents
+    * @returns Observable<void> Completes when all agents are resumed
     */
    resumeAllBuildAgents(): Observable<void> {

Also applies to: 66-75

---

43-52: Consider enhancing the response type.

The bulk operations could benefit from returning information about the number of agents affected. This would help with user feedback and debugging.

-    pauseAllBuildAgents(): Observable<void> {
-        return this.http.put<void>(`${this.adminResourceUrl}/agents/pause-all`, null)
+    interface BulkOperationResult {
+        affectedAgents: number;
+    }
+
+    pauseAllBuildAgents(): Observable<BulkOperationResult> {
+        return this.http.put<BulkOperationResult>(`${this.adminResourceUrl}/agents/pause-all`, null)

-    resumeAllBuildAgents(): Observable<void> {
-        return this.http.put<void>(`${this.adminResourceUrl}/agents/resume-all`, null)
+    resumeAllBuildAgents(): Observable<BulkOperationResult> {
+        return this.http.put<BulkOperationResult>(`${this.adminResourceUrl}/agents/resume-all`, null)

Also applies to: 66-75

src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.ts (2)

98-100: Consider adding type safety for modal parameter.

Instead of using any, consider creating an interface or using NgbModal's types for better type safety.

-displayPauseBuildAgentModal(modal: any) {
+displayPauseBuildAgentModal(modal: NgbModalRef) {

---

Line range hint 1-139: Consider adding comprehensive test coverage.

Given the critical nature of this feature (pausing all build agents), please ensure comprehensive test coverage for:

1. Success and error scenarios for both pause and resume operations
2. Loading states and concurrent operation prevention
3. Modal interaction and proper cleanup
4. Updated build capacity calculation logic

src/test/javascript/spec/component/localci/build-agents/build-agents.service.spec.ts (3)

150-164: Enhance test coverage for bulk operations

While the basic test structure is good, consider enhancing these tests to verify the success path more thoroughly.

 it('should pause all build agents', () => {
-    service.pauseAllBuildAgents().subscribe();
+    service.pauseAllBuildAgents().subscribe(response => {
+      expect(response).toBeDefined();
+      // Add specific expectations based on the expected response
+    });

     const req = httpMock.expectOne(`${service.adminResourceUrl}/agents/pause-all`);
     expect(req.request.method).toBe('PUT');
     req.flush({});
 });

 it('should resume all build agents', () => {
-    service.resumeAllBuildAgents().subscribe();
+    service.resumeAllBuildAgents().subscribe(response => {
+      expect(response).toBeDefined();
+      // Add specific expectations based on the expected response
+    });

     const req = httpMock.expectOne(`${service.adminResourceUrl}/agents/resume-all`);
     expect(req.request.method).toBe('PUT');
     req.flush({});
 });

---

Line range hint 201-233: Strengthen error handling assertions

The error handling tests follow good practices but could be more specific in their assertions.

 it('should handle pause all build agents error', async () => {
     const errorMessage = 'Failed to pause build agents';
+    const statusCode = 500;
+    const statusText = 'Internal Server Error';

     const observable = lastValueFrom(service.pauseAllBuildAgents());

     const req = httpMock.expectOne(`${service.adminResourceUrl}/agents/pause-all`);
     expect(req.request.method).toBe('PUT');
-    req.flush({ message: errorMessage }, { status: 500, statusText: 'Internal Server Error' });
+    req.flush({ message: errorMessage }, { status: statusCode, statusText });

     try {
         await observable;
         throw new Error('expected an error, but got a success');
     } catch (error) {
-        expect(error.message).toContain(errorMessage);
+        expect(error.status).toBe(statusCode);
+        expect(error.statusText).toBe(statusText);
+        expect(error.message).toBe(errorMessage);
     }
 });

Apply similar improvements to the resume all agents error handling test.

---

Line range hint 1-233: Overall test structure follows best practices

The test suite demonstrates good practices:

• Proper use of beforeEach for setup
• Consistent use of HttpTestingController for HTTP mocking
• Comprehensive error handling
• Clear test descriptions
• Proper cleanup with afterEach

Consider adding test cases for:

• Request body validation for PUT requests
• Edge cases like empty responses
• Network timeouts

src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.html (1)

3-21: Consider enhancing button states for better UX.

The layout and organization look good, but consider these UX improvements:

1. Add loading states to buttons during API calls
2. Consider adding aria-labels to buttons for better accessibility
3. Add tooltips to explain the impact of each action

 <button class="btn btn-success" (click)="resumeAllBuildAgents()"
+        [disabled]="isLoading"
+        aria-label="Resume all build agents"
+        tooltip="Resume all paused build agents">
     <fa-icon [icon]="faPlay" />
+    @if (isLoading) {
+        <span class="spinner-border spinner-border-sm" role="status"></span>
+    }
     <span jhiTranslate="artemisApp.buildAgents.resumeAll"></span>
 </button>

src/test/javascript/spec/component/localci/build-agents/build-agent-summary.component.spec.ts (1)

5-5: Enhance mock service responses for better test coverage

The mock responses for pauseAllBuildAgents and resumeAllBuildAgents return empty objects. Consider using more realistic mock responses that match the actual service implementation.

 const mockBuildAgentsService = {
     getBuildAgentSummary: jest.fn().mockReturnValue(of([])),
-    pauseAllBuildAgents: jest.fn().mockReturnValue(of({})),
-    resumeAllBuildAgents: jest.fn().mockReturnValue(of({})),
+    pauseAllBuildAgents: jest.fn().mockReturnValue(of({ success: true, message: 'All agents paused' })),
+    resumeAllBuildAgents: jest.fn().mockReturnValue(of({ success: true, message: 'All agents resumed' })),
 };

Also applies to: 17-17, 31-32

src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.java (1)

Line range hint 198-215: Consider enhancing error handling for edge cases

While the implementation is clean and well-documented, consider adding explicit error handling for cases such as:

• Agent not found
• Agent already paused
• Agent in an invalid state

 @PutMapping("agents/{agentName}/pause")
 public ResponseEntity<Void> pauseBuildAgent(@PathVariable String agentName) {
     log.debug("REST request to pause agent {}", agentName);
-    localCIBuildJobQueueService.pauseBuildAgent(agentName);
-    return ResponseEntity.noContent().build();
+    try {
+        localCIBuildJobQueueService.pauseBuildAgent(agentName);
+        return ResponseEntity.noContent().build();
+    } catch (IllegalArgumentException e) {
+        log.warn("Agent {} not found", agentName);
+        return ResponseEntity.notFound().build();
+    } catch (IllegalStateException e) {
+        log.warn("Cannot pause agent {}: {}", agentName, e.getMessage());
+        return ResponseEntity.badRequest().build();
+    }
 }

src/main/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java (1)

157-167: Consider adding validation and concurrent modification protection

The bulk operations could benefit from additional safeguards:

1. Validate agent states before operations
2. Handle concurrent modifications
3. Return operation results

Consider these architectural improvements:

1. Create a result class to track operation outcomes:

public record BuildAgentOperationResult(
    List<String> successfulAgents,
    List<String> failedAgents,
    List<String> skippedAgents
) {}

2. Enhance the methods to return results and handle concurrency:

public BuildAgentOperationResult pauseAllBuildAgents() {
    log.info("Initiating pause operation for all build agents");
    List<String> successfulAgents = Collections.synchronizedList(new ArrayList<>());
    List<String> failedAgents = Collections.synchronizedList(new ArrayList<>());
    List<String> skippedAgents = Collections.synchronizedList(new ArrayList<>());
    
    getBuildAgentInformation().parallelStream().forEach(agent -> {
        String agentName = agent.buildAgent().name();
        try {
            // Skip if agent is already paused
            if (agent.status() == BuildAgentStatus.PAUSED) {
                skippedAgents.add(agentName);
                return;
            }
            pauseBuildAgent(agentName);
            successfulAgents.add(agentName);
        } catch (Exception e) {
            failedAgents.add(agentName);
            log.error("Failed to pause agent {}: {}", agentName, e.getMessage());
        }
    });
    
    return new BuildAgentOperationResult(successfulAgents, failedAgents, skippedAgents);
}

Configuration used: .coderabbit.yaml
Review profile: CHILL

#!/bin/bash
# Search for any remaining old endpoint patterns
rg -i "api/admin/agent/[^s]" --type ts

#!/bin/bash
# Let's try a broader search to find all agent-related endpoints
rg -i "api/admin/agent" --type ts -A 2

#!/bin/bash
# Also search for the new endpoint pattern to confirm the changes
rg -i "api/admin/agents/" --type ts -A 2

#!/bin/bash
# Let's also check the Angular route configurations
fd -e ts -exec grep -l "agent" {} \; | xargs rg "path.*agent"

#!/bin/bash
# Let's search for the admin resource URL pattern in the codebase
rg -i "adminResourceUrl.*agent" --type ts

#!/bin/bash
# Let's also check the service class implementation
rg -i "class.*BuildAgentsService" --type ts -A 5

#!/bin/bash
# Search for references to buildCapacity in the codebase
rg -A 3 "buildCapacity" --type ts

#!/bin/bash
# Check the test file content to understand the test coverage for buildCapacity calculation
cat src/test/javascript/spec/component/localci/build-agents/build-agent-summary.component.spec.ts

#!/bin/bash
# Search for any monitoring or dashboard components that might use build agent data
rg -g "*.{ts,html}" "(monitoring|dashboard|metrics|capacity).*(build-agent|buildAgent)" -A 3

#!/bin/bash
# Description: Verify the implementation of new methods in SharedQueueManagementService

# Search for the implementation of new methods
ast-grep --pattern 'class SharedQueueManagementService {
  $$$
  public $_ pauseAllBuildAgents() {
    $$$
  }
  $$$
  public $_ resumeAllBuildAgents() {
    $$$
  }
  $$$
}'

#!/bin/bash
# Let's try a different approach to find the service implementation
# First, let's find the LocalCIBuildJobQueueService implementation
rg -l "class LocalCIBuildJobQueueService" 

# Then search for pauseAllBuildAgents method implementation
rg "pauseAllBuildAgents" -A 5

#!/bin/bash
# Description: Check for proper integration with build agent status tracking

# Check for BuildAgentStatus enum usage and status update handling
ast-grep --pattern 'enum BuildAgentStatus {
  $$$
}'

# Check for status update handling in agent management
rg -A 5 'BuildAgentStatus' 

# Check for proper event handling in build agent implementations
rg -A 5 'pauseBuildAgentTopic|resumeBuildAgentTopic'

src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIResourceIntegrationTest.java (2)

367-370: Enhance test coverage for individual agent pause/resume.

Consider adding the following test scenarios:

1. Attempting to pause a non-existent agent
2. Verifying that other agents remain unaffected
3. Verifying the state transition behavior

 @Test
 @WithMockUser(username = TEST_PREFIX + "admin", roles = "ADMIN")
 void testPauseBuildAgent() throws Exception {
     // We need to clear the processing jobs to avoid the agent being set to ACTIVE again
     processingJobs.clear();
 
     request.put("/api/admin/agents/" + URLEncoder.encode(agent1.buildAgent().name(), StandardCharsets.UTF_8) + "/pause", null, HttpStatus.NO_CONTENT);
     await().until(() -> buildAgentInformation.get(agent1.buildAgent().memberAddress()).status() == BuildAgentInformation.BuildAgentStatus.PAUSED);
 
     request.put("/api/admin/agents/" + URLEncoder.encode(agent1.buildAgent().name(), StandardCharsets.UTF_8) + "/resume", null, HttpStatus.NO_CONTENT);
     await().until(() -> buildAgentInformation.get(agent1.buildAgent().memberAddress()).status() == BuildAgentInformation.BuildAgentStatus.IDLE);
+    
+    // Test non-existent agent
+    request.put("/api/admin/agents/non-existent-agent/pause", null, HttpStatus.NOT_FOUND);
+    
+    // Add another agent and verify it remains unaffected
+    var agent2 = new BuildAgentInformation(new BuildAgentDTO("agent2", "address2", "Agent 2"), 1, 0, 
+        new ArrayList<>(), BuildAgentInformation.BuildAgentStatus.IDLE, new ArrayList<>(), null);
+    buildAgentInformation.put("address2", agent2);
+    
+    request.put("/api/admin/agents/" + URLEncoder.encode(agent1.buildAgent().name(), StandardCharsets.UTF_8) + "/pause", null, HttpStatus.NO_CONTENT);
+    await().until(() -> buildAgentInformation.get(agent1.buildAgent().memberAddress()).status() == BuildAgentInformation.BuildAgentStatus.PAUSED);
+    assertThat(buildAgentInformation.get("address2").status()).isEqualTo(BuildAgentInformation.BuildAgentStatus.IDLE);
 }

---

Line range hint 1-391: Consider improving test class organization and documentation.

While the test implementation is solid, consider the following improvements:

1. Add JavaDoc to document test scenarios and edge cases
2. Extract test data setup into dedicated helper methods
3. Add consistent assertion messages for better test failure diagnosis

Example improvements:

+/**
+ * Integration tests for build agent management functionality.
+ * Tests cover:
+ * - Individual agent pause/resume operations
+ * - Bulk agent operations
+ * - Error scenarios and edge cases
+ * - Concurrent operations
+ */
 class LocalCIResourceIntegrationTest extends AbstractProgrammingIntegrationLocalCILocalVCTestBase {
     // ... existing code ...
+    
+    /**
+     * Creates a test build agent with the given configuration.
+     * @param name Agent name
+     * @param address Member address
+     * @param status Initial status
+     * @return Configured BuildAgentInformation
+     */
+    private BuildAgentInformation createTestAgent(String name, String address, 
+            BuildAgentInformation.BuildAgentStatus status) {
+        return new BuildAgentInformation(
+            new BuildAgentDTO(name, address, name),
+            1, 0, new ArrayList<>(), status, new ArrayList<>(), null
+        );
+    }

Configuration used: .coderabbit.yaml
Review profile: CHILL